Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Region] 지역 API 작업 완료 #4

Merged
merged 23 commits into from
Nov 2, 2023
Merged

[Region] 지역 API 작업 완료 #4

merged 23 commits into from
Nov 2, 2023

Conversation

onetuks
Copy link
Contributor

@onetuks onetuks commented Nov 1, 2023

  • 법정동

    • 요청 파라미터 없이 지역 조회 시 조회 가능한 시도 데이터 응답
    • 시도 데이터만 포함하여 지역 조회 요청 시 해당 시도에 해당하는 시군구 데이터 응답
    • 시도, 시군구 데이터로 지역 조회 요청 시 해당 시도, 시군구에 해당하는 읍면동 데이터 응답
  • 위치

    • 시도, 시군구, 읍면동 데이터로 위치 정보 요청 시 해당 위치에 해당하는 위경도 데이터 응답

@onetuks onetuks self-assigned this Nov 1, 2023
@onetuks onetuks added the enhancement New feature or request label Nov 1, 2023
wonu606
wonu606 previously approved these changes Nov 2, 2023

@Id
@Column(name = "id", nullable = false)
private Long id;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아이디 직접 넣어주시나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

원우님 이거 실수가 아니라 의도한 거였습니다.
데이터를 넣어주는 역할은 StudayServer 가 아니라 Studay_Data_Invocator 이고,
이쪽에서 그저 받아서 쓰기만 하기 때문에 @column으로 생성 자체를 막아두려는 의도입니다.

근데 저희 지금 테스트 특성 상 데이터를 넣어줄 필요가 있으니 setter 메소드가 따로 필요하거나 생성자가 따로 필요할 것 같은데 이 부분에 관해서 조금 고민이 필요할 것 같습니다.

@@ -7,7 +7,7 @@ HELP.md
.gradle/
build/
studay_server.iml
src/main/resources/application.yaml
*.yaml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

헉.. 저도 설정했는데 나중에 conflict 처리해야겠군요 ㅠㅠ

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ㅠㅠ 컨플릭트 싫은뎅 ㅠㅠ

import org.springframework.web.bind.annotation.RestController;

@RestController
@RequestMapping(path = "/regions", produces = APPLICATION_JSON_VALUE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 지금 produces 메서드에서 사용하고 있는데 이렇게 변경할까요?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메서드단으로 합의!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오케!!!

Comment on lines 29 to 40
@GetMapping(path = "/beopjungdong")
public ResponseEntity<RegionResponse> getSubRegions(
@RequestParam(required = false) String sido,
@RequestParam(required = false) String sigungu
) {
if (sido == null) {
return getSidoData();
} else if (sigungu == null) {
return getSigunguData(sido);
}
return getUpmyeondongData(sido, sigungu);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

밑에 구현 여러 개 잘해놓으셔서
required = false 처리 안해도 괜찮지 않을까 싶은데
required = false로 하신 이유가 있을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1안 (현재안)
/regions/beopjungdong?sido=XX&sigungu=XX

2안
/regions/beopjungdong/sido?sido=XX
/regions/beopjungdong/sigungu?sido=XX&sigungu=XX

byeolhaha
byeolhaha previously approved these changes Nov 2, 2023
Copy link
Member

@byeolhaha byeolhaha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

세영님 고생하셨습니다!

코드 깔끔해서 너무 쉽게 쉽게 읽혔어요

@@ -28,7 +28,14 @@ public ResponseEntity<RegionResponse> getSigungus(
@RequestParam(required = false) String sido,
@RequestParam(required = false) String sigungu
) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

서비스 단으로 가기 전에 끝에 커스텀 어노테이션을 통해서 "시도"로 끝나는지 "시군구"로 끝나는지 확인해 보는 것은 어떨까요? @ValidSido, @ValidSigungu

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 좋네요!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ValidSido : 서울특별시, 경기도 아니면 날림 (null 인 건 괜찮음)
@ValidSigungu : 시, 군, 구 포함되지 않으면 날림 (Null 인 건 괜찮음)
@ValidUpmyeondong : 읍, 면, 동, 군, 구 포함되지 않으면 날림

wonu606
wonu606 previously approved these changes Nov 2, 2023
@onetuks onetuks dismissed stale reviews from wonu606 and byeolhaha via fbab695 November 2, 2023 15:46
@wonu606 wonu606 merged commit f09d3c1 into develop Nov 2, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants